Fix crash on continue after a missing package non-critical error #5942
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Due to the way how exceptions propagate from the Anaconda installation tasks we ended
up with a non-critical error exception interrupting the iteration of an important task
iteration loop installing the payload.
Due to not being handled "deep enough" but instead "bubbling up" to a top level
error handler the loop apparently gets interrupted & remaining tasks
skipped.
This resulted in an unrelated crash ("kernel version list no available")
due to all the important installation tasks being skipped, packages not
being installed and installation related data not being populated.
The end result was that a non-critical error (such as a missing package)
would trigger a dialog asking the user to quit or continue - but clicking
"continue" would result in a weird crash.
So move the error handler check closer to the task execution, to
prevent the loop from being interrupted. That way the loop will resume
its iteration if "continue" is clicked in the UI.
Also convert the non-critical error if it gets raised (user deciding
*not* to continue after the non-critical error) to a fatal one. This is
necessary, as otherwise the top level error handler would get triggered,
asking the user again to quit or continue.
NOTE: Longer term we really should clean this up & have all installation
tasks gathered, ordered and executed from a single place. Then all the
error handling could be in a single place, making things much simpler.
(cherry picked from commit 73b412f)
Resolves: RHEL-57699
Resolves: INSTALLER-4045
Related: rhbz#2238045
|
/kickstart-test --testtype smoke |
1 similar comment
|
/kickstart-test --testtype smoke |
|
/kickstart-test --skip-testtypes whatever |
|
Wow, all 120 test passed with 5 retries - seems to be working, thanks @rvykydal! :) I've also checked the results c for retried tests both sets of results are there, so if we wanted to look for race conditions, we still can do that even with retries on. ) |
KKoukiou
reviewed
Oct 22, 2024
| # This results in a nice error dialog with "Exit Installer" button | ||
| # being shown. | ||
| raise PayloadInstallationError(str(e)) from e | ||
| else: |
There was a problem hiding this comment.
Hm I dont get why we need to check fro ERROR_RAISE, handle broad Exception and raise again.
If we dont handle it it will be just propagate up. What am I missing here?
I mean convert this to::
try:
sync_run_task(task_proxy)
except NonCriticalInstallationError as e:
# Handle the non-critical error by raising it as a fatal PayloadInstallationError
raise PayloadInstallationError(str(e)) from e
M4rtinK
added
the
ready to merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Due to the way how exceptions propagate from the Anaconda installation tasks we ended up with a non-critical error exception interrupting the iteration of an important task iteration loop installing the payload.
Due to not being handled "deep enough" but instead "bubbling up" to a top level error handler the loop apparently gets interrupted & remaining tasks skipped.
This resulted in an unrelated crash ("kernel version list no available") due to all the important installation tasks being skipped, packages not being installed and installation related data not being populated.
The end result was that a non-critical error (such as a missing package) would trigger a dialog asking the user to quit or continue - but clicking "continue" would result in a weird crash.
So move the error handler check closer to the task execution, to prevent the loop from being interrupted. That way the loop will resume its iteration if "continue" is clicked in the UI.
Also convert the non-critical error if it gets raised (user deciding not to continue after the non-critical error) to a fatal one. This is necessary, as otherwise the top level error handler would get triggered, asking the user again to quit or continue.
NOTE: Longer term we really should clean this up & have all installation tasks gathered, ordered and executed from a single place. Then all the error handling could be in a single place, making things much simpler.
(cherry picked from commit 73b412f)
Resolves: RHEL-57699
Resolves: INSTALLER-4045
Related: rhbz#2238045
Backport of #5937 to the RHEL 10 branch.